Skip to content

gh-119670: Add force keyword only argument to shlex.quote#148846

Open
jb2170 wants to merge 15 commits intopython:mainfrom
jb2170:shlex-quote-force
Open

gh-119670: Add force keyword only argument to shlex.quote#148846
jb2170 wants to merge 15 commits intopython:mainfrom
jb2170:shlex-quote-force

Conversation

@jb2170
Copy link
Copy Markdown
Contributor

@jb2170 jb2170 commented Apr 21, 2026 •
edited
Loading

Closes #119670
Supersedes my out-of-date PR #119674

We use force instead of always as a word more consistent with other programming jargon (like in rm -f the f is for 'force') as discussed

Commits

  • ddbe3a5: Add base implementation.
  • 0e722d6: Make shlex.quote's kwargs kwargs-only. Backwards compatible. There are propositions to add a single-quote-double-quote switch, which I'm ambivalent about, but to future-proof against hiccups of people passing force as a positional and it ending up being used as the wrong kwarg, we make the kwargs kwargs-only, eg like json.loads.
  • fd4af18: Add tests. I've tried to be pretty thorough. Their purposes are commented on.
  • 78b6f3b: Update docs. Contains explanation and examples.
  • 762999d: Add blurb entry.
  • 2a301a5: Add whatsnew entry.

As mentioned I'm also going to open an issue a discussion thread about shlex.quote returning "''" for falsey non-str data. This PR and that discussion seem independent of each other, that is this PR doesn't need to be held back until that discussion is addressed.

📚 Documentation preview 📚: https://cpython-previews--148846.org.readthedocs.build/

jb2170 added 5 commits April 21, 2026 04:41
@jb2170
shlex: Implement force parameter behavior for shlex.quote
ddbe3a5
@jb2170
shlex: Make force a keyword only argument in shlex.quote
0e722d6 
There are propositions to add a single-quote-double-quote switch
(pythongh-90630), so to avoid hiccups of people passing `force` as a
positional and it being used for the single-double switch, we make
kwargs kwargs-only.
@jb2170
shlex tests: Add testForceQuote
fd4af18 
Test special cases of strings that don't need quoting, do need quoting,
do use `force`, don't use `force` etc. I've tried to be exhaustive.
@jb2170
shlex: Update documentation to mention shlex.quote's force kwarg
78b6f3b
@jb2170
Add blurb entry for pythongh-119670: Add force keyword only argument …
762999d 
…to `shlex.force`
@bedevere-app bedevere-app Bot mentioned this pull request Apr 21, 2026
Open
@jb2170 jb2170 mentioned this pull request Apr 21, 2026
Closed
@jb2170 jb2170 requested a review from AA-Turner as a code owner April 21, 2026 18:43
@jb2170
Copy link
Copy Markdown
Contributor Author

jb2170 commented Apr 21, 2026

@vstinner pong 😅

johnslavik
johnslavik requested changes Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@johnslavik johnslavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment thread Lib/shlex.py Outdated
Comment thread Lib/test/test_shlex.py Outdated
Comment thread Lib/test/test_shlex.py Outdated
@johnslavik
Copy link
Copy Markdown
Member

johnslavik commented Apr 22, 2026 •
edited
Loading

Hmm I also have a hunch that this might be worth calling out in What's New. Nvm it's already there, great job 🚀

jb2170 and others added 4 commits April 22, 2026 18:59
@jb2170 @johnslavik
shlex.quote: improve formatting
f470aa1 
Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
@jb2170 @johnslavik
shlex.quote tests: Remove comments
5450159 
The comments for why these tests exist can always be found in the git history

Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
@jb2170
shlex.quote tests: Remove another comment
bcd8b63 
Missed from last commit
@jb2170 @johnslavik
shlex tests: Remove misplaced comment
d000ccd 
Intended for another branch shlex-quote-typeerror

Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
@jb2170
Copy link
Copy Markdown
Contributor Author

jb2170 commented Apr 24, 2026

Looks ready (for review again?) to me 😎

vstinner
vstinner approved these changes Apr 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@picnixz: Do you want to double check? You reviewed the previous PR gh-119674.

@picnixz picnixz self-requested a review April 27, 2026 14:03
picnixz
picnixz reviewed Apr 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor tweaks but please add more tets with quotes inside.

Comment thread Doc/library/shlex.rst Outdated
Comment thread Doc/library/shlex.rst Outdated
Comment thread Doc/library/shlex.rst
Comment thread Doc/whatsnew/3.15.rst Outdated
Comment thread Lib/shlex.py Outdated
Comment thread Lib/shlex.py Outdated
Comment thread Lib/shlex.py Outdated
Comment thread Lib/test/test_shlex.py
Comment thread Misc/NEWS.d/next/Library/2026-04-21-06-30-59.gh-issue-119670.pMWZfY.rst Outdated
@jb2170 jb2170 force-pushed the shlex-quote-force branch from a9e06cd to e7bf496 Compare April 28, 2026 07:44
@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 28, 2026

Please avoid force-pushing. It's extremely hard to make reviews otherwise.

jb2170 added 2 commits April 28, 2026 09:17
@jb2170
Tweak shlex.quote comments based on picnixz's review
cd5d9c2 
Try not to use don't and we're and should've etc apostrophes.
@jb2170
Tweak shlex.quote tests based on picnixz's review
e3d9b05 
Add test to ensure string with single quote inside it (without which the
string wouldn't need quoting) is quoted even though `force=False`.
@jb2170 jb2170 force-pushed the shlex-quote-force branch from e7bf496 to e3d9b05 Compare April 28, 2026 08:38
@jb2170
Copy link
Copy Markdown
Contributor Author

jb2170 commented Apr 28, 2026 •
edited
Loading

Sorry for the force pushes but the 'commit suggestions' commits on GitHub came through as CRLF formatted instead of LF. I don't know if that's a GitHub Microsoft issue, or your IDE settings, or I need to git config autocrlf but hey I got there in the end.

$ git log d000ccd...e3d9b05 is what has changed, addressing the review. The only change I disagreed with is the wording of the .. versionadded. The existing one seems to be worded more similarly to the others.

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 28, 2026

I think it's a GH issue because other reviews had the same issue...

@jb2170
Copy link
Copy Markdown
Contributor Author

jb2170 commented Apr 28, 2026

Ready

@picnixz picnixz self-requested a review April 28, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johnslavik johnslavik johnslavik requested changes

@vstinner vstinner vstinner approved these changes

@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

@picnixz picnixz Awaiting requested review from picnixz

Assignees

No one assigned

Labels

awaiting merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

shlex.quote: Add force keyword argument

4 participants

@jb2170 @johnslavik @picnixz @vstinner